codemod iterations#2386
Merged
Merged
Conversation
🦋 Changeset detectedLatest commit: 85879d7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/core
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
felixweinberger
requested changes
Jun 29, 2026
felixweinberger
left a comment
Contributor
There was a problem hiding this comment.
Needs a CI check - seems to fail a build
- typedoc: exclude src/bin from codemod docs so the batch-test harness's
exported types no longer trip treatWarningsAsErrors (fixes check:all)
- runner: shift diagnostic lines by the restored shebang's line count so
reported file:line matches the saved file
- batchTest: record post-install versions in a separate map so a
startup-unresolved package isn't retroactively pinned for later repos
- batchTest/README: correct npm-view abort wording, the parseNpmViewVersion
'highest match' comment, and 'How it works' step 4
- changeset: patch @modelcontextprotocol/codemod for the shebang fix
…tprotocol/typescript-sdk into feature/codemod-iterations-4
felixweinberger
previously approved these changes
Jun 29, 2026
Contributor
Author
|
@claude review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat(codemod): test the batch harness against the published codemod + SDK, and preserve shebangs
Extends the codemod batch-test harness so it can validate the published
@modelcontextprotocol/codemodand the published v2 SDK packages from npm — not just the local working copy — and fixes a codemod bug that dropped a file's leading#!shebang during migration.Motivation and Context
The batch-test harness runs the v1→v2 codemod against real-world repos to catch regressions, missing transforms, and gaps. Until now it could only exercise the local working-copy codemod against locally-packed SDK tarballs. That validates the branch under development, but never the artifacts users actually
npm install— so a bug present only in a published build, or an incompatibility with a published SDK alpha, would slip through.This PR adds independent source selection for the codemod and the SDK, each
localorpublished, plus version pinning:--codemodlocal|publishedlocallocal: run the working-copy codemod in-process.published:npxthe published CLI.--sdklocal|publishedlocallocal: pack local packages, rewrite deps tofile:tarballs.published: install v2 deps from npm.--codemod-version <spec>latest--codemod=published.--sdk-version <spec>--sdk=published. Pins each v2 dep to its own resolved version; unset = whatever the codemod writes.So the same harness can now answer "does the published codemod + published SDK still migrate these repos cleanly?", with results written to per-run directories keyed on the resolved versions.
Separately, several transforms consumed a file's leading
#!shebang (it is leading trivia of the first import they rewrite), silently breaking CLI packages whosebinpoints at the migrated entry. The runner now captures the shebang before transforms and restores it before saving.How Has This Been Tested?
test/batchTest.test.ts— arg parsing (--flag valueand--flag=value,--separator, invalid/unknown flags, ignored version overrides),parseNpmViewVersion,parseCodemodCliOutput,computeResultsDirNameacross all modes,rewriteToPublishedVersion(per-package independent versions, formatting/trailing-newline preserved),cleanSubprocessEnv, and localrunCodemod.test/integration.test.tsverifies a migrated CLI file keeps#!/usr/bin/env node(plus its following blank line);test/commentInsertion.test.tsverifies a comment-bearing diagnostic's reportedfile:linepoints at the actual line in the saved (shebang-restored) file.repos.json(punkpeye/fastmcp,upstash/context7,firebase/firebase-tools) in both local and published modes.pnpm check:allis green (typecheck + lint +docs:check); the full codemod suite passes (411 tests).Breaking Changes
None. The batch-test harness is dev-only tooling. The shebang fix is a user-facing codemod bugfix (output is now more correct) shipped as a
patchchangeset for@modelcontextprotocol/codemod; no consumer action required.Types of changes
Checklist
Additional context
Results layout. Each run writes to
batch-test/results/codemod-…__sdk-…/, keyed on the resolved versions, with a top-levelsummary.json(per-repo pass/fail, error counts, runconfig, and the resolved per-packagesdkVersions) and areport.jsonper repo (baseline vs post-codemod check results, diagnostics, change counts). In published-codemod mode the CLI emits text rather than structured diagnostics, socodemod.diagnosticsis empty and the raw CLI output is captured undercodemod.cli.Subprocess env hygiene. The harness is invoked via
pnpm, which exports its own config asnpm_config_*/PNPM_*vars; those leak into every subprocess and break things (minimum-release-age blocks installs, workspace-cwdnpxmis-resolves).cleanSubprocessEnvstrips them so installs,npx, andnpm viewsee a clean package-manager env (npmrc files still honored).Security. The
npx/npm view/gitshell-outs interpolate only operator-controlled inputs — CLI flags, the committedrepos.json, and Anthropic-published npm versions.JSON.stringifyquoting does not neutralize$(…)/backticks undersh -c, so the harness must never be pointed at an untrusted manifest. This is called out at each call site.Review fixes folded in (latest commit):
typedoc.jsonexcludes the dev-onlysrc/bin/**so the harness's exported types stop tripping typedoc'streatWarningsAsErrors(this was failingdocs:check, hencecheck:all).npm view"aborts the run" wording (a per-package--sdk-versionmiss only warns) and theparseNpmViewVersioncomment (npm lists matches in publish order, not semver order).